Create BATS tests for limactl-mcp #4060

jandubois · 2025-09-20T21:51:47Z

I had an idea how to test limactl-mcp with BATS, so I wrote this PoC. I think it works well, but must obviously be fleshed out some more.

This PR depends on #3744 and assumes it has been merged already.

jandubois · 2025-09-20T21:55:48Z

One thing I found while writing the test was that run_shell_command returns an object that has additional JSON embedded as a string:

$ echo '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"run_shell_command","arguments":{"command":["cat","os-release"],"directory":"/etc"}}}' >&"${MCP[1]}"

$ read -t 1 -r line <&"${MCP[0]}"; jq . <<<"$line"
{
  "jsonrpc": "2.0",
  "id": 3,
  "result": {
    "content": [
      {
        "type": "text",
        "text": "{\"stdout\":\"PRETTY_NAME=\\\"Ubuntu 25.04\\\"\\nNAME=\\\"Ubuntu\\\"\\nVERSION_ID=\\\"25.04\\\"\\nVERSION=\\\"25.04 (Plucky Puffin)\\\"\\nVERSION_CODENAME=plucky\\nID=ubuntu\\nID_LIKE=debian\\nHOME_URL=\\\"https://www.ubuntu.com/\\\"\\nSUPPORT_URL=\\\"https://help.ubuntu.com/\\\"\\nBUG_REPORT_URL=\\\"https://bugs.launchpad.net/ubuntu/\\\"\\nPRIVACY_POLICY_URL=\\\"https://www.ubuntu.com/legal/terms-and-policies/privacy-policy\\\"\\nUBUNTU_CODENAME=plucky\\nLOGO=ubuntu-logo\\n\",\"stderr\":\"\",\"exit_code\":0}"
      }
    ]
  }
}

So you need to extract .text, unmarshall the result into JSON and then extract .stdout, which is quite cumbersome. I don't know if this will bother an LLM, but doesn't this also increase the number of tokens used for the content by escaping all the quotes and backslashes?

Is there a reason this can't be a normal nested object?

jandubois · 2025-09-21T04:13:59Z

Beyond the encoded JSON thing, I don't understand the schema of the output: Why is content an array? I don't understand how there can be more than one set of results given that the tool only accepts a single command.

And what does type: text mean? Especially given that text is a json string. Could it also be object? Isn't this all too complex for a tool command?

So I would expect the output of run_shell_command to look more like this:

{
  "jsonrpc": "2.0",
  "id": 3,
  "result": {
    "stdout": "...",
    "stderr": "",
    "exit_code": 0
  }
}

jandubois · 2025-09-22T05:14:49Z

The glob and list_directory tools seem redundant:

If you make the pattern for glob optional and have it default to *, then list_directory is just the same as glob.

AkihiroSuda · 2025-09-22T09:09:36Z

So I would expect the output of run_shell_command to look more like this:

MCP doesn't allow that schema: https://pkg.go.dev/github.com/modelcontextprotocol/go-sdk@v0.6.0/mcp#CallToolResult

StructuredContent may be useful for a similar schema, but it might be relatively new and maybe not understood by some AI agents.

https://modelcontextprotocol.io/specification/2025-06-18/server/tools#structured-content also says "For backwards compatibility, a tool that returns structured content SHOULD also return the serialized JSON in a TextContent block."

I guess we can revisit this later.

The glob and list_directory tools seem redundant:

The current design is to follow Gemini's design
https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/shell.md

jandubois · 2025-09-23T03:49:07Z

I have more questions:

The glob description says:

The absolute path to the directory to search within. If omitted, searches the tool's root directory.

What is the tool's root directory? How is the LLM to know what this means?

If this command accepts relative patterns, why do the other tools insist on an absolute path? Why can't you pass a filename returned by glob to read_file? How do you turn the relative file into an absolute file?

The description for the run_in_shell command says:

The exact shell command to execute. Defined as a string slice, unlike Gemini's run_shell_command that defines it as a single string.

I think the negative explanation "this is not like something else" is not helpful. Also most LLMs will not even know what the Gemini run_shell_command does.

Describe what the tool does, and what parameters it expects in a prescriptive manner.

The write_file tool does not create the directory for the file to be written.

It returns an error file does not exist. I think it should create missing directories. But if it doesn't, the error should be directory does not exist.

AkihiroSuda · 2025-09-23T07:45:26Z

Opened an issue about StructuredContent for tracking purpose:

MCP: switch from JSON TextContent to StructuredContent #4070

Please consider opening separate issues for other topics too.

What is the tool's root directory?

The prompt was just copied from https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/file-system.md#4-glob-findfiles
Needs to be reworded that it refers to the cwd of the agent, which should correspond to the cwd of the MCP tools too.

If this command accepts relative patterns, why do the other tools insist on an absolute path?

The prompt was just copied from Gemini, but glob seems different from other tools in the sense that it is specifically made for resolving a vague path pattern to an exact pattern.

Why can't you pass a filename returned by glob to read_file?

Bug. glob should return absolute paths that can be passed to read_file.
https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/file-system.md#4-glob-findfiles

How do you turn the relative file into an absolute file?

Maybe we should provide an additional MCP tool, if shelling out realpath does not work.

I think the negative explanation "this is not like something else" is not helpful. Also most LLMs will not even know what the Gemini run_shell_command does.

Gemini may know?

The write_file tool does not create the directory for the file to be written.

Bug.

jandubois · 2025-09-24T00:51:08Z

Why can't you pass a filename returned by glob to read_file?

Bug. glob should return absolute paths that can be passed to read_file.

I must have been confused about this; I can no longer reproduce the issue and get full path names now. I probably mixed it up with the results of list_directory, which has relative paths because the directory name is a required parameter.

AkihiroSuda · 2025-10-01T08:38:08Z

Needs rebase after merging:

mcp: use StructuredContent; fix "path is empty" error #4119

Signed-off-by: Jan Dubois <jan.dubois@suse.com>

jandubois · 2025-10-10T05:24:16Z

I think this is good enough for a first pass. There is always more that could be covered, but there is at least some coverage now for each tool.

There are 2 skipped tests that should be enabled when the bugs #4173 and #4174 have been fixed.

AkihiroSuda · 2025-10-10T05:25:21Z

hack/bats/tests/mcp.bats

+    limactl delete --force "$NAME" || :
+    # Make sure that the host agent doesn't inherit file handles 3 or 4.
+    # Otherwise bats will not finish until the host agent exits.
+    limactl start --yes --name "$NAME" template://default 3>&- 4>&-


Not necessarily needs to be covered in this PR, but we should consider reusing an instance across multiple bats files

lima/hack/bats/tests/preserve-env.bats

Line 20 in 8b1f07d

limactl start --yes --name "$NAME" template:default 3>&- 4>&-

Yes, absolutely.

Just for working on the tests you can already do this:

❯ export LIMA_BATS_REUSE_INSTANCE=1 ❯ ./hack/bats/lib/bats-core/bin/bats -T ./hack/bats/tests/mcp.bats -f search_file mcp.bats ✓ search_file_content finds text inside files [507] ✓ search_file_content can find unicode characters above U+FFFF [510] ✓ search_file_content returns an empty string if it cannot find the pattern [502] 3 tests, 0 failures in 2 seconds ❯ ./hack/bats/lib/bats-core/bin/bats -T ./hack/bats/tests/mcp.bats -f search_file mcp.bats ✓ search_file_content finds text inside files [516] ✓ search_file_content can find unicode characters above U+FFFF [512] ✓ search_file_content returns an empty string if it cannot find the pattern [501] 3 tests, 0 failures in 3 seconds

So you can iterate quickly without restarting the instance every time.

And both mcp.bats and preserve-env.bats are sharing the same bats instance, so it works for both.

Nice, the code clone across the bats files should be moved to helpers

jandubois · 2025-10-10T05:41:01Z

I don't know if #4124 should be considered a bug or not. Right now this test assumes the current scheme is the way it is supposed to be.

jandubois · 2025-10-10T05:52:18Z

hack/bats/tests/mcp.bats

+    tools_call glob '{"pattern":"nothing.to.see"}'
+
+    run_yq '.structuredContent.matches[]' <<<"$output"
+    assert_output_lines_count 0


This may need to be changed to refute_output because I think $lines is undefined and not an empty array when there is no output. Can be fixed when the test is no longer skipped.

AkihiroSuda

Thanks

jandubois force-pushed the bats-mcp branch 4 times, most recently from f56729d to 1f10d05 Compare September 21, 2025 03:27

jandubois force-pushed the bats-mcp branch 5 times, most recently from 576a149 to 5118e73 Compare September 22, 2025 05:07

AkihiroSuda added area/test Tests and CI area/mcp labels Sep 22, 2025

jandubois force-pushed the bats-mcp branch 2 times, most recently from 19e7ded to 3c04a67 Compare September 23, 2025 03:42

jandubois force-pushed the bats-mcp branch from 3c04a67 to fa1c77a Compare September 23, 2025 03:55

AkihiroSuda added this to the v2.0.0 milestone Sep 23, 2025

jandubois force-pushed the bats-mcp branch 2 times, most recently from a6c02cb to 0387659 Compare September 23, 2025 05:25

jandubois force-pushed the bats-mcp branch 2 times, most recently from f5e8cbb to 0ebb19a Compare September 25, 2025 01:32

jandubois force-pushed the bats-mcp branch 2 times, most recently from c4e9a81 to fefc8fe Compare October 2, 2025 05:18

Create BATS tests for limactl-mcp

8b1f07d

Signed-off-by: Jan Dubois <jan.dubois@suse.com>

jandubois force-pushed the bats-mcp branch from fefc8fe to 8b1f07d Compare October 10, 2025 05:21

jandubois marked this pull request as ready for review October 10, 2025 05:21

AkihiroSuda reviewed Oct 10, 2025

View reviewed changes

jandubois commented Oct 10, 2025

View reviewed changes

AkihiroSuda approved these changes Oct 16, 2025

View reviewed changes

AkihiroSuda merged commit e41492a into lima-vm:master Oct 16, 2025
37 checks passed

Create BATS tests for limactl-mcp #4060

Create BATS tests for limactl-mcp #4060

Uh oh!

Conversation

jandubois commented Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jandubois commented Sep 20, 2025

Uh oh!

jandubois commented Sep 21, 2025

Uh oh!

jandubois commented Sep 22, 2025

Uh oh!

AkihiroSuda commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jandubois commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda commented Sep 23, 2025

Uh oh!

jandubois commented Sep 24, 2025

Uh oh!

AkihiroSuda commented Oct 1, 2025

Uh oh!

jandubois commented Oct 10, 2025

Uh oh!

AkihiroSuda Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

jandubois Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

jandubois commented Oct 10, 2025

Uh oh!

jandubois Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jandubois commented Sep 20, 2025 •

edited

Loading

AkihiroSuda commented Sep 22, 2025 •

edited

Loading

jandubois commented Sep 23, 2025 •

edited

Loading

jandubois Oct 10, 2025 •

edited

Loading